Overhaul needless_parens_on_range_literals
#15717
Conversation
rustbot
added
the
S-waiting-on-review
| //~^ needless_parens_on_range_literals | ||
|
|
||
| // lint: the macro doesn't have anything to do with the paren | ||
| let _ = 0..1; |
There was a problem hiding this comment.
not sure why verbatim!(0) is replaced with just 0 here, given that I use snippet_with_context
There was a problem hiding this comment.
Ah, that's because literal is 0 just here, which doesn't come from expansion -- the verbatim! macro takes it from its input and, well, returns it verbatim. Because of that, things like literal.span.source_callsite() don't help either. I'd appreciate some help^^
There was a problem hiding this comment.
If you run Clippy here you can check that the tokens are the same, even the syntax context (the number after the #).
So if the macro is just ($t:tt) => ($t), Clippy and the compiler overall will ignore it, plus (even worse) don't even know it's there.
That's why we cannot detect such cases, sadly.
There was a problem hiding this comment.
Hm.. I wanted to change the test to something like:
macro_rules! zero {
() => {
0
};
}
let _ = (zero!())..1;But I'm not even sure we want to lint that? Because it might very well be that zero! instead expanded to something like 1 - 1, in which case its output would need parenthesization (what a word). This kind of comes back to #15724 (comment), but my grammar knowledge is nowhere nearly good enough to implement something like that^^
I guess not bothering to lint this particular case is always an option as well. WDYT?
rustbot
removed
the
S-waiting-on-review
rustbot
added
the
S-waiting-on-review
Sadly, we don't really want to make lints early unless there's a good reason. Late lint passes have better support, so we can make better utilities for them (+ they're filtered, so they don't incur any performance impact necessarily) |
Huh! Firstly, this is again something I've heard people voice the opposite opinion on12 -- though [1] in particular was made in the context of attribute-related lints, in which case late-pass is really not the right approach. I'd appreciate it if a consensus was reached on this, and ideally documented somewhere. Also, fwiw the lint did start its life as early-pass, but was changed to late-pass during development in order to be able to detect unsuffixed float literals ending with a Still, given the considerations you voiced, I could turn it back to late-pass (wouldn't be too complicated). WDYT? Footnotes |
This comment has been minimized.
This comment has been minimized.
ada4a
force-pushed
the
needless_parens_on_range_literals
branch
from
d04a02a to
fc1a9ef
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Since I needed to rebase onto master anyway, I went ahead and made the lint late-pass again, and removed the problematic macro test case |
3 participants
This PR makes the lint early-pass, and fixes the following issues:
changelog: [
needless_parens_on_range_literals]: Fix FN on unsuffixed floats not ending with a.changelog: [
needless_parens_on_range_literals]: Fix FP on macro-generated code